Ensure we do not try to inline async versions of pinvokes#129797
Open
jakobbotsch wants to merge 2 commits into
Open
Ensure we do not try to inline async versions of pinvokes#129797jakobbotsch wants to merge 2 commits into
jakobbotsch wants to merge 2 commits into
Conversation
We normally fail inlining of pinvokes when we call getMethodInfo, but if we tried inlining an async variant of a pinvoke then that call would succeed and we would end up breaking in getEHinfo that does not handle this case.
Contributor
|
Tagging subscribers to this area: @agocke |
jakobbotsch
commented
Jun 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CoreCLR JIT/EE interface to consistently use the ordinary (non-async-variant) MethodDesc as the source of IL/EH metadata when a method participates in async-variant codegen, preventing the JIT from treating async variants of P/Invokes as inlineable IL methods and later asserting when EH info is requested.
Changes:
- Add
MethodDesc::GetOrdinaryVariantIfAsyncVersion()to normalize async-variant methods to their ordinary variant where appropriate. - Extend
CEEInfo::getMethodInfoWorkerto take an explicitilFtn(the method whose IL/header should be used) and update call sites accordingly. - Update
getMethodInfo, acanInlineEH-count check path, andCEECodeGenInfo::getEHinfoto use the ordinary variant for IL/EH sourcing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/vm/method.hpp | Adds a helper to map async-variant methods to their ordinary variant for IL/EH sourcing. |
| src/coreclr/vm/jitinterface.h | Updates getMethodInfoWorker signature to accept an explicit IL-source MethodDesc. |
| src/coreclr/vm/jitinterface.cpp | Threads ilFtn through method-info/EH-info paths to avoid inlining/metadata mismatches for async P/Invoke variants. |
jakobbotsch
commented
Jun 24, 2026
| result = true; | ||
| } | ||
| else if (ftn->IsIL() || ftn->IsDynamicMethod()) | ||
| else if (ilFtn->IsIL() || ilFtn->IsDynamicMethod()) |
Member
Author
There was a problem hiding this comment.
The change to use ilFtn in this check is the actual fix for the problem, the rest is just avoiding multiple GetOrdinaryVariant calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We normally fail inlining of pinvokes when we call getMethodInfo, but if we tried inlining an async variant of a pinvoke then that call would succeed and we would end up breaking in getEHinfo that does not handle this case.
Fix #129782